-
Notifications
You must be signed in to change notification settings - Fork 803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix PVF precompilation for Kusama #5606
Conversation
The CI pipeline was cancelled due to failure one of the required jobs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function name could be more descriptive, other than that: Looks good.
|
||
// There is still a chance to be a previous session authority, but this extra work does not | ||
// affect the finalization. | ||
is_past_present_or_future_authority && !is_present_authority | ||
is_past_present_or_future_authority && !is_present_validator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This distinction exists until we remove the max_validators
configuration so even without this fix it still works as expected on Polkadot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at what check_next_session_authority
, I understand this change, but it seems to conflate multiple cases in a not-ideal way.
My understanding is that after restart we want to check if we are a (para)validator in the current or the next session, and if so, precompile PVFs.
Also, if we are already running and are not a (para)validator in the current session, but are in the next, we also want to precompile PVFs.
What this function does is checks for (not-only-para)validator set of current and next session along with a paravalidator set of past dispute_period
sessions. It doesn't distinguish fresh start from already running and checks if we are not current session (with this change para)validator.
Ideally, we'd have a separate runtime API to tell us if we are in the next (para)validator set, but we don't have that currently, so I understand this is a workaround solution. We could also probably simplify the check to just look at the next set assuming that if we already have the artifacts cached, pre-compilation will be a no-op.
Missing prdoc |
Because on Kusama validators.len() < discovery_keys.len() we can tweak the PVF precompilation to allow prepare PVFs when the node is an authority but not a validator.